feat: add discord > matrix reaction support #862

SethFalco · 2022-10-09T14:23:38Z

Notes

There are a few ways we can approach bridging reactions with custom emojis.


name	But on Discord custom emotes can have the same name, so if the same user does multiple reactions with emojis with the same name, it'll only come through as one. If they do 2 reactions, then remove one, Discord will show the other, while Matrix will show neither.
name:id	But on Discord, people can rename emotes. So if they react, then rename the emote, then remove the reaction, it will count as a different emote and won't be removed on the Matrix side.
id	But on Matrix, this is basically a meaningless number, at that point we might as well not bridge custom emojis at all.

It looks like in future it should be supported to use external images as reactions via MXC URLs. I don't do this yet because the shortcode is not exposed in the SDK and it's not clearly documented behavior, but in future can update the bridge to support that.

Based on what I've seen, others use the format :name: for the shortcode property. Based on this, I've decided to use the same format already. That way when we're in a position to support MXC URLs and the shortcode property, we can just move the value of key to shortcode for those cases.

Screenshots

A message and 3 reactions all sent from Discord.

How the same message/reactions appear on Matrix when bridged.

Notes

getRelationsForEvent was moved out of unstableApis in the main branch for matrix-bot-sdk. Eventually, when we update to a version of matrix-appservice-bridge that uses that version we should switch it out stop using it from unstableApis.

tezlm · 2022-10-09T21:47:30Z

discord custom emojis can be done by setting a m.relates_to key to an mxc:// url and content-level shortcode to the shortcode

{
    "shortcode": "something",
    "m.relates_to": {
        "event_id": "$abc123",
        "key": "mxc://servername/someidhere",
        "rel_type": "m.annotation"
    }
}

SethFalco · 2022-10-10T07:26:32Z

That's interesting. The MXC URL can be done already, but the SDK doesn't expose the shortcode property.

I'll leave this as-is for now, but I'll check what you said and verify if that's actually how it works. If so, I can open a PR to matrix-bot-sdk to include this, then come back to fix it here in a 2nd iteration if that's cool with maintainers.

Any chance you can link to where doing images as reactions is documented/defined?

dali99 · 2022-10-11T10:28:44Z

I believe the closest thing to documentation on that is the MSC: matrix-org/matrix-spec-proposals#3746

A bigger argument is probably that mx-puppet-discord, cinny, fluffychat, beeper, some element patches floating about, and probably more clients, all special case seeing an mxc url and render it as an image

src/bot.ts

src/db/dbdataevent.ts

mangofeet · 2023-02-15T17:32:20Z

I was testing this and it all seems to be working fine, but I'm getting this error

Exception thrown while handling "messageReactionRemove" event MatrixError: M_FORBIDDEN: User @_discord_<discord id>:example.com not in room !<room id>:example.com, and room previews are disabled

The room in question is a private room that has the bridge bot in it with admin access. Despite this error, everything appears to be functioning correctly.

SethFalco · 2023-02-15T22:07:39Z

Hey @mangofeet, thanks for testing this!

Any chance you could provide more in-depth steps to reproduce or a stack trace?
If not, I'll get my environment up when I have time and try to check this out.

salixor · 2023-05-24T20:18:42Z

Hi there ! I'm currently using this change on two servers with a few people. Thanks a lot for it !

I have two minor issues with it.

The first one, as @mangofeet reported, is that we are getting the same errors :

Exception thrown while handling "messageReactionAdd" event MatrixError: M_FORBIDDEN: User @_discord_<id>:<homeserver> not in room <room>
Exception thrown while handling "messageReactionRemove" event MatrixError: M_FORBIDDEN: User @_discord_<id>:<homeserver> not in room <room>, and room previews are disabled

A way to replicate the issue :

bridge a channel
have a Discord user which never posted in this channel since the briding react on a message
have the same user remove their reaction (still without sending any message)
the reaction is not bridged and the error is found in logs

Shouldn't the ghost be invited before trying to create a reaction on Matrix's side ?

The second issue has to do with Discord bot users. It seems their reactions are not bridged at all. And any subsequent Discord user which tries to react will simply be ignored. This specific emoji can never be seen on Matrix side. Having a Discord user use another emoji to react does work.

src/bot.ts

SethFalco · 2023-07-06T00:16:13Z

@salixor @mangofeet I've rebased with develop, and resolved the exceptions when users aren't in the room when they add/remove reactions now.
I'd appreciate it if you could test it, just to confirm everything is good. 👍

I also found another bug. Removing all reactions in the Discord interface should bridge to Matrix correctly now.

Signed-off-by: Seth Falco <[email protected]>

salixor · 2023-07-14T22:03:11Z

Hey there, haven't had much time last week to try the changes out, but I haven't forgotten !

I'm glad to say the first issue is solved !

The second issue is still a weird one.
The reactions have been bridged on one message, but not the other - with no apparent error.
On the first message, adding the same reaction myself removes the reaction of the Discord bot user on Matrix's side, as shown below :

It's a very niche use case however, so I'd say it works well enough. At least actual user reactions are always bridged, which is the most important imo.

For the multiple room issue, we only stumbled on this use case because of an old room which bridge was kinda jumbled, and I don't really know how to replicate the same use case - sorry ! Also a niche use case, and the changes seem reasonable.

Really hope it gets merged someday, and thanks a lot for the work !
Gonna run my bridge with these new changes, and I'll report if something weird happens.

PS : I tried the remove all feature but it doesn't seem to interface properly. I get no errors but the reaction stay on Matrix's side

GitMensch · 2024-03-07T11:42:01Z

Is there anything open but the rebase (same question goes for the related #877)?
Who would handle the merge?

chayleaf · 2024-06-11T14:27:42Z

src/bot.ts

+                    return false;
+                }
+
+                return event.content["m.relates_to"].key === reaction.emoji.name;


you should use reactionName here in the same way as in OnMessageReactionAdd

chayleaf · 2024-06-11T14:30:56Z

test/test_discordbot.ts

+            discordBot = getDiscordBot();
+            const intent = mockBridge.getIntent(author.id);
+
+            intent.underlyingClient.unstableApis.getRelationsForEvent = async () => {


this is not unstable anymore, using it via .unstableApis is deprecated

Suggested change

intent.underlyingClient.unstableApis.getRelationsForEvent = async () => {

intent.underlyingClient.getRelationsForEvent = async () => {

chayleaf · 2024-06-11T14:31:04Z

src/bot.ts

+
+            const underlyingClient = intent.underlyingClient;
+
+            const { chunk } = await underlyingClient.unstableApis.getRelationsForEvent(


Suggested change

const { chunk } = await underlyingClient.unstableApis.getRelationsForEvent(

const { chunk } = await underlyingClient.getRelationsForEvent(

chayleaf · 2024-06-11T14:31:13Z

src/bot.ts

+            const [ eventId, roomId ] = storeEvent.MatrixId.split(";");
+            const underlyingClient = this.bridge.botIntent.underlyingClient;
+
+            const { chunk } = await underlyingClient.unstableApis.getRelationsForEvent(


Suggested change

const { chunk } = await underlyingClient.unstableApis.getRelationsForEvent(

const { chunk } = await underlyingClient.getRelationsForEvent(

chayleaf · 2024-06-11T14:31:32Z

test/mocks/appservicemock.ts

+    public async getRelationsForEvent(roomId: string, eventId: string, relationType?: string, eventType?: string): Promise<any> {
+        this.funcCalled("getRelationsForEvent", roomId, eventId, relationType, eventType);
+    }


move to MatrixClientMock

SethFalco · 2024-06-11T14:34:04Z

I've actually deleted my Discord account since creating this PR.
Would anyone care to take it off my hands to continue maintaining it?

Edit: I won't close it unless someone wants to take over. In future, I may create a dummy account just to update the PRs if no one else wants to maintain it.

SethFalco requested a review from a team as a code owner October 9, 2022 14:23

SethFalco force-pushed the reactions branch from 429c0a8 to 8301ad6 Compare October 9, 2022 14:26

SethFalco mentioned this pull request Oct 9, 2022

docs: fix typos in jsdocs turt2live/matrix-bot-sdk#268

Merged

3 tasks

SethFalco force-pushed the reactions branch 2 times, most recently from 37819f5 to 2e91853 Compare October 9, 2022 15:17

SethFalco force-pushed the reactions branch from 2e91853 to 4b032cf Compare October 11, 2022 10:05

SethFalco force-pushed the reactions branch 2 times, most recently from 92c3151 to ecc9dca Compare October 20, 2022 14:08

SethFalco force-pushed the reactions branch from ecc9dca to 9b3533f Compare November 17, 2022 09:35

tadzik requested changes Jan 27, 2023

View reviewed changes

SethFalco force-pushed the reactions branch 2 times, most recently from 2a2548d to 20f9048 Compare February 5, 2023 12:11

mangofeet mentioned this pull request Feb 14, 2023

feat: add Matrix -> Discord reactions #877

Open

4 tasks

SethFalco requested a review from tadzik February 15, 2023 10:40

salixor reviewed Jun 9, 2023

View reviewed changes

src/bot.ts Outdated Show resolved Hide resolved

SethFalco force-pushed the reactions branch 3 times, most recently from ba3d10b to 2ac843f Compare July 6, 2023 00:47

feat: add discord > matrix reaction support

3106957

Signed-off-by: Seth Falco <[email protected]>

SethFalco force-pushed the reactions branch from 2ac843f to b3d7b41 Compare July 6, 2023 00:48

SethFalco added 3 commits July 6, 2023 01:49

refactor: improve logs and avoid bad requests

a818687

Signed-off-by: Seth Falco <[email protected]>

fix: correctly handle multiple room bridges

da2c3b8

Signed-off-by: Seth Falco <[email protected]>

fix: join room on reaction events

a33f269

Signed-off-by: Seth Falco <[email protected]>

SethFalco force-pushed the reactions branch from b3d7b41 to a33f269 Compare July 6, 2023 00:49

Vyquos mentioned this pull request Dec 3, 2023

reactions to both sides? #903

Open

chayleaf suggested changes Jun 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add discord > matrix reaction support #862

feat: add discord > matrix reaction support #862

SethFalco commented Oct 9, 2022 •

edited

Loading

tezlm commented Oct 9, 2022

SethFalco commented Oct 10, 2022 •

edited

Loading

dali99 commented Oct 11, 2022

mangofeet commented Feb 15, 2023

SethFalco commented Feb 15, 2023

salixor commented May 24, 2023 •

edited

Loading

SethFalco commented Jul 6, 2023 •

edited

Loading

salixor commented Jul 14, 2023 •

edited

Loading

GitMensch commented Mar 7, 2024

chayleaf Jun 11, 2024

chayleaf Jun 11, 2024

chayleaf Jun 11, 2024

chayleaf Jun 11, 2024

chayleaf Jun 11, 2024

SethFalco commented Jun 11, 2024 •

edited

Loading

	intent.underlyingClient.unstableApis.getRelationsForEvent = async () => {
	intent.underlyingClient.getRelationsForEvent = async () => {


		const underlyingClient = intent.underlyingClient;

		const { chunk } = await underlyingClient.unstableApis.getRelationsForEvent(

feat: add discord > matrix reaction support #862

Are you sure you want to change the base?

feat: add discord > matrix reaction support #862

Conversation

SethFalco commented Oct 9, 2022 • edited Loading

Related

Notes

Screenshots

Notes

tezlm commented Oct 9, 2022

SethFalco commented Oct 10, 2022 • edited Loading

dali99 commented Oct 11, 2022

mangofeet commented Feb 15, 2023

SethFalco commented Feb 15, 2023

salixor commented May 24, 2023 • edited Loading

SethFalco commented Jul 6, 2023 • edited Loading

salixor commented Jul 14, 2023 • edited Loading

GitMensch commented Mar 7, 2024

chayleaf Jun 11, 2024

Choose a reason for hiding this comment

chayleaf Jun 11, 2024

Choose a reason for hiding this comment

chayleaf Jun 11, 2024

Choose a reason for hiding this comment

chayleaf Jun 11, 2024

Choose a reason for hiding this comment

chayleaf Jun 11, 2024

Choose a reason for hiding this comment

SethFalco commented Jun 11, 2024 • edited Loading

SethFalco commented Oct 9, 2022 •

edited

Loading

SethFalco commented Oct 10, 2022 •

edited

Loading

salixor commented May 24, 2023 •

edited

Loading

SethFalco commented Jul 6, 2023 •

edited

Loading

salixor commented Jul 14, 2023 •

edited

Loading

SethFalco commented Jun 11, 2024 •

edited

Loading